Properly parse ISO date strings by fjakobs · Pull Request #473 · databricks/databricks-sdk-py

fjakobs · 2023-12-05T13:03:05Z

Pre Python 3.11 the datetime.fromisoformat isn't a generic date time parser and can only be used to parse ISO dates generated by the python datatime module. This PR normalized the date string the same way the google oauth2 library does by discarding the timezone and millisecond part of the date string.

Changes

Make date parsing for tokens from the CLI more robust

Tests

Added unit test
Manually tested with CLI auth provider

codecov-commenter · 2023-12-06T08:30:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5955333) 57.53% compared to head (ca4964a) 57.53%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #473   +/-   ##
=======================================
  Coverage   57.53%   57.53%           
=======================================
  Files          38       38           
  Lines       25554    25555    +1     
=======================================
+ Hits        14703    14704    +1     
  Misses      10851    10851

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mgyucht

Small nit on the tests but otherwise LGTM.

mgyucht · 2023-12-06T11:54:03Z

tests/test_core.py

+    CliTokenSource._parse_expiry("2023-12-01T15:19:48.007742617Z")
+    CliTokenSource._parse_expiry("2023-12-05T15:59:01.40081+11:00")
+    CliTokenSource._parse_expiry("2023-12-06 10:06:05")


Can you just add an assertion on the actual returned values?
BTW: good practice here would be to use pytest.mark.parameterize annotation, e.g.

@pytest.mark.parametrize( "date_string,expected", [("2023-12-01T15:19:48.007742617Z", <the expected date>), ("2023-12-05T15:59:01.40081+11:00", <the expected date>), ("2023-12-06 10:06:05", <the expected date>)]) def test_databricks_cli_token_parse_expiry(date_string, expected):

Bugfixes: * Fixed accidental rename ([#471](#471)). * Fixed parsing of ISO date strings ([#473](#473)). Other changes: * Updated GCP OAuth Readme ([#464](#464)). * Reference Documentation Refactoring ([#467](#467)). * Installed local library when generating docs ([#469](#469)). * Fixed readme links in pypi ([#472](#472)). * Updated a note for installing Python SDK on Databricks Runtime 13.1+ ([#474](#474)). * Updated GCP auth readme ([#470](#470)). API Changes: * Changed `update()` method for [w.connections](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/connections.html) workspace-level service with new required argument order. * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.CreateStorageCredential`. * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.StorageCredentialInfo`. * Changed `name` field for `databricks.sdk.service.catalog.UpdateCatalog` to be required. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateCatalog`. * Changed `name` field for `databricks.sdk.service.catalog.UpdateConnection` to no longer be required. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateConnection`. * Changed `name` field for `databricks.sdk.service.catalog.UpdateExternalLocation` to be required. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateExternalLocation`. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateMetastore`. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateRegisteredModelRequest`. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateSchema`. * Changed `name` field for `databricks.sdk.service.catalog.UpdateStorageCredential` to be required. * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.UpdateStorageCredential`. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateStorageCredential`. * Added `new_name` field for `databricks.sdk.service.catalog.UpdateVolumeRequestContent`. * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.ValidateStorageCredential`. * Added `databricks.sdk.service.catalog.CloudflareApiToken` dataclass. * Removed `continuous` field for `databricks.sdk.service.jobs.BaseRun`. * Removed `continuous` field for `databricks.sdk.service.jobs.Run`. * Changed `job_parameters` field for `databricks.sdk.service.jobs.RunJobTask` to `databricks.sdk.service.jobs.ParamPairs` dataclass. * Added `run_if` field for `databricks.sdk.service.jobs.SubmitTask`. * Added `run_job_task` field for `databricks.sdk.service.jobs.SubmitTask`. * Changed `update_config()` method for [w.serving_endpoints](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/serving_endpoints.html) workspace-level service with new required argument order. * Added `put()` method for [w.serving_endpoints](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/serving_endpoints.html) workspace-level service. * Added `rate_limits` field for `databricks.sdk.service.serving.CreateServingEndpoint`. * Changed `served_models` field for `databricks.sdk.service.serving.EndpointCoreConfigInput` to no longer be required. * Added `auto_capture_config` field for `databricks.sdk.service.serving.EndpointCoreConfigInput`. * Added `served_entities` field for `databricks.sdk.service.serving.EndpointCoreConfigInput`. * Added `auto_capture_config` field for `databricks.sdk.service.serving.EndpointCoreConfigOutput`. * Added `served_entities` field for `databricks.sdk.service.serving.EndpointCoreConfigOutput`. * Added `served_entities` field for `databricks.sdk.service.serving.EndpointCoreConfigSummary`. * Added `served_entities` field for `databricks.sdk.service.serving.EndpointPendingConfig`. * Added `extra_params` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `input` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `max_tokens` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `messages` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `n` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `prompt` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `stop` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `stream` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Added `temperature` field for `databricks.sdk.service.serving.QueryEndpointInput`. * Changed `predictions` field for `databricks.sdk.service.serving.QueryEndpointResponse` to no longer be required. * Added `choices` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Added `created` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Added `data` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Added `id` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Added `model` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Added `object` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Added `usage` field for `databricks.sdk.service.serving.QueryEndpointResponse`. * Changed `workload_size` field for `databricks.sdk.service.serving.ServedModelInput` to `databricks.sdk.service.serving.ServedModelInputWorkloadSize` dataclass. * Changed `workload_type` field for `databricks.sdk.service.serving.ServedModelInput` to `databricks.sdk.service.serving.ServedModelInputWorkloadType` dataclass. * Added `task` field for `databricks.sdk.service.serving.ServingEndpoint`. * Added `task` field for `databricks.sdk.service.serving.ServingEndpointDetailed`. * Added `databricks.sdk.service.serving.Ai21LabsConfig` dataclass. * Added `databricks.sdk.service.serving.AnthropicConfig` dataclass. * Added `databricks.sdk.service.serving.AutoCaptureConfigInput` dataclass. * Added `databricks.sdk.service.serving.AutoCaptureConfigOutput` dataclass. * Added `databricks.sdk.service.serving.AutoCaptureState` dataclass. * Added `databricks.sdk.service.serving.AwsBedrockConfig` dataclass. * Added `databricks.sdk.service.serving.AwsBedrockConfigBedrockProvider` dataclass. * Added `databricks.sdk.service.serving.ChatMessage` dataclass. * Added `databricks.sdk.service.serving.ChatMessageRole` dataclass. * Added `databricks.sdk.service.serving.CohereConfig` dataclass. * Added `databricks.sdk.service.serving.DatabricksModelServingConfig` dataclass. * Added `databricks.sdk.service.serving.EmbeddingsV1ResponseEmbeddingElement` dataclass. * Added `databricks.sdk.service.serving.EmbeddingsV1ResponseEmbeddingElementObject` dataclass. * Added `databricks.sdk.service.serving.ExternalModel` dataclass. * Added `databricks.sdk.service.serving.ExternalModelConfig` dataclass. * Added `databricks.sdk.service.serving.ExternalModelProvider` dataclass. * Added `databricks.sdk.service.serving.ExternalModelUsageElement` dataclass. * Added `databricks.sdk.service.serving.FoundationModel` dataclass. * Added `databricks.sdk.service.serving.OpenAiConfig` dataclass. * Added `databricks.sdk.service.serving.PaLmConfig` dataclass. * Added `databricks.sdk.service.serving.PayloadTable` dataclass. * Added `databricks.sdk.service.serving.PutRequest` dataclass. * Added `databricks.sdk.service.serving.PutResponse` dataclass. * Added `databricks.sdk.service.serving.QueryEndpointResponseObject` dataclass. * Added `databricks.sdk.service.serving.RateLimit` dataclass. * Added `databricks.sdk.service.serving.RateLimitKey` dataclass. * Added `databricks.sdk.service.serving.RateLimitRenewalPeriod` dataclass. * Added `databricks.sdk.service.serving.ServedEntityInput` dataclass. * Added `databricks.sdk.service.serving.ServedEntityOutput` dataclass. * Added `databricks.sdk.service.serving.ServedEntitySpec` dataclass. * Added `databricks.sdk.service.serving.ServedModelInputWorkloadSize` dataclass. * Added `databricks.sdk.service.serving.ServedModelInputWorkloadType` dataclass. * Added `databricks.sdk.service.serving.V1ResponseChoiceElement` dataclass. * Removed [a.account_network_policy](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_network_policy.html) account-level service. * Removed `databricks.sdk.service.settings.AccountNetworkPolicyMessage` dataclass. * Removed `databricks.sdk.service.settings.DeleteAccountNetworkPolicyRequest` dataclass. * Removed `databricks.sdk.service.settings.DeleteAccountNetworkPolicyResponse` dataclass. * Removed `databricks.sdk.service.settings.ReadAccountNetworkPolicyRequest` dataclass. * Removed `databricks.sdk.service.settings.UpdateAccountNetworkPolicyRequest` dataclass. * Removed `name` field for `databricks.sdk.service.sharing.UpdateCleanRoom`. * Changed `name` field for `databricks.sdk.service.sharing.UpdateProvider` to be required. * Added `new_name` field for `databricks.sdk.service.sharing.UpdateProvider`. * Changed `name` field for `databricks.sdk.service.sharing.UpdateRecipient` to be required. * Added `new_name` field for `databricks.sdk.service.sharing.UpdateRecipient`. * Changed `name` field for `databricks.sdk.service.sharing.UpdateShare` to be required. * Added `new_name` field for `databricks.sdk.service.sharing.UpdateShare`. * Added `statement_ids` field for `databricks.sdk.service.sql.QueryFilter`. * Added `databricks.sdk.service.sql.StatementId` dataclass. OpenAPI SHA: 63caa3cb0c05045e81d3dcf2451fa990d8670f36, Date: 2023-12-12

fjakobs requested a review from mgyucht December 5, 2023 13:03

fjakobs force-pushed the ES-959603 branch 3 times, most recently from 8acd84d to 8a6a1e9 Compare December 6, 2023 08:28

fjakobs force-pushed the ES-959603 branch 2 times, most recently from ea75f44 to b62ea96 Compare December 6, 2023 08:42

fjakobs changed the title ~~Use "fromisoformat" to parse tokens from the CLI~~ Properly parse ISO date strings Dec 6, 2023

fjakobs force-pushed the ES-959603 branch from 872cf90 to dfb78ce Compare December 6, 2023 09:23

fjakobs added 3 commits December 6, 2023 10:25

Use "fromisoformat" to parse tokens from the CLI

8e070ff

Use dateutil for ISO time parsing

b267307

Copy code from google OAuth credentials.py

47fe244

fjakobs force-pushed the ES-959603 branch from dfb78ce to 47fe244 Compare December 6, 2023 09:25

fjakobs enabled auto-merge December 6, 2023 09:25

mgyucht approved these changes Dec 6, 2023

View reviewed changes

fjakobs added this pull request to the merge queue Dec 6, 2023

mgyucht removed this pull request from the merge queue due to a manual request Dec 6, 2023

fjakobs added 2 commits December 7, 2023 13:14

Assert return values

56cc374

Merge branch 'main' into ES-959603

ca4964a

fjakobs enabled auto-merge December 7, 2023 12:15

fjakobs added this pull request to the merge queue Dec 7, 2023

Merged via the queue into main with commit 741238b Dec 7, 2023

fjakobs deleted the ES-959603 branch December 7, 2023 12:18

hectorcast-db mentioned this pull request Dec 12, 2023

Release v0.15.0 #478

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Properly parse ISO date strings#473

Properly parse ISO date strings#473
fjakobs merged 5 commits intomainfrom
ES-959603

fjakobs commented Dec 5, 2023 •

edited

Loading

Uh oh!

codecov-commenter commented Dec 6, 2023 •

edited

Loading

Uh oh!

mgyucht left a comment

Uh oh!

mgyucht Dec 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

fjakobs commented Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Uh oh!

codecov-commenter commented Dec 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

mgyucht Dec 6, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fjakobs commented Dec 5, 2023 •

edited

Loading

codecov-commenter commented Dec 6, 2023 •

edited

Loading